-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revamp D65 chroma correction workflow #15461
Revamp D65 chroma correction workflow #15461
Conversation
And denoise profile. @rawfiner thoughts? |
Exactly :-) This would be a prerequisite ... |
ec4684d
to
27cbe27
Compare
@wpferguson indeed, denoise profile in Y0U0V0 mode is working better with an accurate white balance |
27cbe27
to
a329052
Compare
Just tested. Great improvement on denoise profiled :-D. I have a directory of Play Raws and I('ve gotten 2 crashes trying to import it, so there's something going on. I'll see if I can track it down. |
Leave this for a day or two. Will need more care. |
It appears to be choking on RAF files. Is there something different about Fuji white balance? |
a329052
to
e6bc729
Compare
@jenshannoschwalm, not your bug. See #15477 |
Here is constant work on this so force pushing what is stable working yet. Next will be proposal for white balance improved. So far working good with better trouble messages and infrastructure only... |
7fd0433
to
ef3eb85
Compare
Ok, from my side it seems to be working fine all over, would love
There are now 4 commits 1&2 introduce the new chroma struct, some refactoring and make sure the module trouble message work fine. So something i suggest to get reviewed @TurboGit 3&4 introduce the simplistic user interface, temperature module version required bumping (required both for user interface and for cache safety without playing with iop cache data). Implementation in colorin needs more love for sure, will follow soon if tests go fine. |
Reading the log messages I found
Did you mean color calibration module instances? I'm testing now. Do you have any particular test cases you want me to try? |
YES FOR SURE! channelmixerrgb or color calibration, just fixed the commit message :-) THANKS! |
ef3eb85
to
4b61b10
Compare
Nope. Whatever and However your workflow is. Modules resulting in a visible difference will mostly be those mentioned above. |
I was commenting about this too... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First review on the code.
@@ -1622,6 +1661,7 @@ void reload_defaults(dt_iop_module_t *module) | |||
dt_bauhaus_combobox_add(g->presets, C_("white balance", "user modified")); | |||
// old "camera neutral", reason: better matches intent | |||
dt_bauhaus_combobox_add(g->presets, C_("white balance", "camera reference")); | |||
dt_bauhaus_combobox_add(g->presets, C_("white balance", "late camera reference")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need more comment about this as I'm not sure what it represents exactly and will have trouble during translation to French. Maybe a better wording for this new option is possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tooltip added below is:
set white balance to as shot and later correct to camera reference point
But then what is the difference between camera reference
and late camera reference
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree and the wording is just "preliminary". I am not even sure we want the extra button at all. From my understanding - and if all reviewers and testers agree on this - i would
- keep old behaviour for current image developments
- use new algo by selecting the reset to default
- use new algo for new edits.
But all are open for discussion.
45eca0d
to
5e77ade
Compare
Tested highlight reconstruction, raw ca and denoise profiled. Highllight reconstruction worked without issue in all modes. It seemed to me that I might have got a little more detail in the recovered highlights, but it could be just my imagination. raw ca worked well. I tested against the roof image in the demosiacing improvement issue, as well as a couple of others. raw ca either cleaned up all of the ca or most of it. For the most of it instances applying the ca module finished it up. denoise profiled was night and day for me. I shoot lots of high ISO images (sports, bad lights, high shutter speeds). denoise profiled in master leaves white "noise" artifacts in the dark areas and doesn't always do a really good job of denoising the lighter areas, because it doesn't have access to the white balance coefficients. So after denoising, I apply an instance of D or S with fine denoise to get rid of the artifacts, and then maybe a second instance (either denoise medium or denoise coarse) to smooth out some of the luma noise. This process yields a fairly good result and keeps the room warm :). With this PR the dark areas have no white artifacts, and the lighter areas have some luma noise on really high ISO images. One instance of D or S cleans up the luma noise if I need to. The resulting image is noticeably better than what I can currently achieve from master. |
I tested on some high iso pictures and I hardly see any difference. How did you test? |
From theory in highlights the blown out parts should be corrected more to white instead of yellowish. For raw ca correction the New stuff seems almost always better. |
One more experience, also the LMMSE algo works slightly better. |
While unrelated to this PR wrt your comment...the default slider for preserve shadows I have found leads to a lot of that... simply dropping that back in a measured way really helps.... likely you have explored this but if not then you could check... |
It appears that I tested apples against oranges. I used a high ISO image that I had previously developed and compared the noise reduction of that to the PR. I failed to take into account the other steps that I used developing the image in the first place which had an effect on the output. Once I developed them the same and the only difference was the noise reduction, the difference was much closer. I am glad you asked the question and made me go back and look my method because in that testing I found a nasty little bug unrelated to this PR. Issue coming soon. |
Since introducing the modern chroma correction workflow we do the chroma correction in two phases. First we use the daylight/D65 coeffs in temperature Second we refine for better data in colorbalancergb. Unfortunately 1) some modules would prefer better white-balanced data, notably highlights, raw chromatic aberration as their algorithms assume white-is-white / resp all rgb signals are equal for greytones. 2) due to the implementation in temperature we don't have all coeffs (currently used, D65 and as-shot) publicly available but only internal in gui data. 3) warnings are difficult to handle This pr implements and uses a struct `dt_dev_chroma_t` in `dt_develop_t` holding all relevant data. The temperature module reads them all and we can check at all stages of the pipe via dt_dev_check_d65(), also we avoid keeping coeffs in temperature gui data. Some refactoring and renaming for readability
All chroma related warnings will be served via the channelmixerrgb (color calibration) module instances. The one being `chroma.adaptation` also knows about the temperature module status, so it writes the trouble status there too. For increased responsivness and situations without UI updates (initial loading in darkroom) we update the troubles after the preview pipe has finished.
We now have a second bulb button on the very right introducing the "late D65" mode. The effect will be: 1. the temperature module will process data with "as-shot" coeffs and keeps track on this in the dev chroma struct. 2. also the processed_max and dsc data are using this 3. See colorin module how this get's resolved Otherwise the algos have not changed. Presets are supported as before
Preliminary code for evaluation of the algo 1. only cpu code so far 2. some performance penalty
5e77ade
to
5f06467
Compare
How does this interact with custom WB coefficients? I'm on stable, so can't test it, but I can share a raw and my coefficients. |
Short answer is no, i suppose you mean having presets for temperature. This pr changes the pixelpipe color maths only for the new "D65-late" button. All modules until colorin will use the as-shot coeffs (those are often - not always - better than the known D65 coeffs) and are finally fixed to D65 in the input stage of colorin. So no interference for other workflows. |
Not sure we are talking about the same thing, although I do use a preset. What I've done is create custom D65 coefficients by taking a photo of a calibrated white screen, and used that to get what you see in the attached screenshot. Without, my photos have a pronounced yellow tone, but with these coefficients color calibration CAT is pretty much spot-on.
So, if I understand correctly, there will be a new "D65-late" option (in addition to the current four) in |
#15505 has replaced this so closing |
With the modern scene-referred workflows we do chroma correction in three parts:
This leads to improved color data in follwing modules - thus it was introduced.
Unfortunately there are some downsides from this approach.
Some modules want accurate white balance data for best quality, most notably
highlights and raw ca correction.
Both modules modify data found in the rgb channels towards "best guessed white" but can't
do so as those data are currently not available.
This PR implements a
chroma struct in dev (like a proxy) holding all required and available white balance data for
the current image (D65, as_shot and currently used) allowing runtime
a) check - are we using D65?
b) access of modules to the other coeffs for improved results
c) easier and safe checks for chroma correction related trouble messages
EDIT: (25.10) temperature modules gets a button introducing this mode and colorin does the "late correction".
Fixes #14518
Related to #15121 and discussions about highlights & opposed